feat/forward-search-params #676
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @anyulled, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on enhancing the ticket purchasing flow by including UTM parameters in the ticket links. It also refactors the logic for determining whether certain actions (like buying tickets, submitting CFP, or applying for sponsorship) are enabled based on date intervals, using a new useDateInterval hook. This hook centralizes the date comparison logic and provides boolean flags for disabling the respective buttons.
Highlights
- UTM Parameter Forwarding: The ticket purchase links now include UTM parameters from the current URL, allowing for better tracking of marketing campaign effectiveness.
- Date Interval Hook: Introduces a
useDateIntervalhook to encapsulate the logic for checking if tickets, CFP, and sponsorship applications are within their respective active date ranges. This hook returns boolean flags to disable/enable the corresponding buttons. - Component Refactoring: The
ActionButtonscomponents for the 2023, 2024, and 2025 home pages have been updated to use theuseDateIntervalhook and to pass thedisabledprop to theButtoncomponent based on the hook's return values. The date comparison logic has been removed from the components themselves.
Changelog
Click here to see the changelog
- src/2023/Home/components/ActionButtons/ActionButtons.tsx
- Imported the
useDateIntervalhook. - Replaced the individual date calculations and
isBetweenfunction with theuseDateIntervalhook. - Modified the ticket link to include
window.location.searchto forward UTM parameters. - Added the
disabledprop to the buttons, controlled by theuseDateIntervalhook.
- Imported the
- src/2024/Home/components/ActionButtons.tsx
- Imported the
useDateIntervalhook. - Replaced the individual date calculations and
isBetweenfunction with theuseDateIntervalhook. - Modified the ticket link to include
window.location.searchto forward UTM parameters. - Added the
disabledprop to the buttons, controlled by theuseDateIntervalhook.
- Imported the
- src/hooks/useDateInterval.ts
- Created a new hook
useDateIntervalthat takes a date and the edition data as input. - Calculates whether the current date is within the ticket, CFP, and sponsorship date ranges.
- Returns an object with boolean flags
isTicketsDisabled,isCfpDisabled, andisSponsorDisabled.
- Created a new hook
- src/views/Home/components/ActionButtons/ActionButtons.tsx
- Imported the
useDateIntervalhook. - Replaced the individual date calculations and
isBetweenfunction with theuseDateIntervalhook. - Modified the ticket link to include
window.location.searchto forward UTM parameters. - Added the
disabledprop to the buttons, controlled by theuseDateIntervalhook.
- Imported the
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A hook is born, a function's grace,
To check the dates in time and space.
Tickets, CFP, and sponsors too,
Their buttons wait, for me and you.
UTM's passed, a marketer's dream,
To track the source, it would seem.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new hook useDateInterval to determine if certain dates are within a specified interval, and it also forwards search parameters to the ticket link. The changes look good overall, but there are a few areas that could be improved for clarity and maintainability.
Summary of Findings
- Code Duplication: The logic for determining if dates are within an interval was previously duplicated across multiple components. The introduction of the
useDateIntervalhook addresses this, which is great. - Readability: The removal of the
isBetweenfunction and the date calculations within theActionButtonscomponents improves readability by extracting this logic into a custom hook. - Maintainability: Using
window.location.searchdirectly in thelinkprop might lead to issues if the search parameters are not properly encoded or if there are conflicts with existing parameters. Consider using a utility function to handle URL parameter manipulation.
Merge Readiness
The pull request is almost ready for merging. Addressing the maintainability concern regarding URL parameter manipulation would make the code more robust. I am unable to directly approve the pull request, and other reviewers should review and approve this code before merging.
✅ Deploy Preview for dev-bcn ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces a new useUrlBuilder hook and useDateInterval hook to handle URL construction and date interval checks for the DevBcn website. The goal is to include UTM parameters in ticket links and manage the disabled state of action buttons based on date intervals. The changes involve refactoring the ActionButtons components for different years (2023, 2024, 2025) to use these new hooks.
Summary of Findings
- Dependency Management: The pull request includes changes to
package-lock.jsonandpackage.json, moving@faker-js/fakerfrom dependencies to devDependencies and updating versions of testing libraries. This is good for keeping production dependencies lean. - Code Duplication: The
ActionButtonscomponents for different years (2023, 2024, 2025) are very similar. Consider creating a single, reusable component with props to handle the year-specific data and logic. - Testing: The
useUrlBuilderhook is tested by mockingwindow.location. While this works, consider using a more robust testing strategy that doesn't rely on global mocks.
Merge Readiness
The pull request is almost ready for merging. The introduction of the useUrlBuilder and useDateInterval hooks improves the maintainability and testability of the code. However, addressing the code duplication in the ActionButtons components and improving the testing strategy for useUrlBuilder would make the pull request even better. I am unable to approve this pull request, and recommend that others review and approve this code before merging. Given the medium severity comment, I recommend that the author address it before merging.
include utm params in tickets link